[python-package] enforce keyword-only args in more internal functions #7111
Conversation
jameslamb
changed the title
jameslamb
requested review from
StrikerRUS,
borchero,
guolinke,
jmoralez and
shiyu1994
as code owners
StrikerRUS
left a comment
StrikerRUS
left a comment
There was a problem hiding this comment.
Thanks! Just one minor question below
python-package/lightgbm/callback.py
Outdated
| self.first_metric = "" | ||
|
|
||
| def _gt_delta(self, curr_score: float, best_score: float, delta: float) -> bool: | ||
| def _gt_delta(self, curr_score: float, best_score: float, *, delta: float) -> bool: |
There was a problem hiding this comment.
Just curious, why not after self?
There was a problem hiding this comment.
I was thinking that a binary "op" type of function should always begin with 2 positional arguments, just something I'm used to from other languages.
But I see this is only called by lightgbm's own code (not passed through anything from functools or something) and is called like any other normal function:
LightGBM/python-package/lightgbm/callback.py
Line 416 in 33e93d6
Seeing that, I think keyword arguments there would improve clarity. I'll make that change (and for _lt_delta).
There was a problem hiding this comment.
Updated in 2e48184
I had to add a # type: ignore[call-arg] comment. I think mypy is confused by the use of partial() here:
LightGBM/python-package/lightgbm/callback.py
Line 386 in 33e93d6
python-package/lightgbm/callback.py:416: error: Unexpected keyword argument "curr_score" [call-arg]
python-package/lightgbm/callback.py:416: error: Unexpected keyword argument "best_score" [call-arg]
There was a problem hiding this comment.
Thanks for making calls of these functions even more clear.
Contributes to #3756
Enforces keyword-only arguments in more internal functions, to make the Python codebase a bit stricter and easier to follow.